Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated examples to plot PCA with Roassal 3 #321

Merged
merged 10 commits into from
Jan 18, 2024
Merged

Updated examples to plot PCA with Roassal 3 #321

merged 10 commits into from
Jan 18, 2024

Conversation

hernanmd
Copy link
Contributor

This needs PolyMathOrg/DataFrame#237 to be merged.
Roassal

@hernanmd hernanmd self-assigned this May 25, 2023
@hernanmd hernanmd requested a review from jecisc May 25, 2023 18:07
@jecisc
Copy link
Member

jecisc commented May 29, 2023

Polymath has no dependency on Roassal and I don't think it is a good idea to depend on a visualization framework for a math framework.

Maybe @olekscode had an opinion?

@jordanmontt
Copy link
Contributor

I agree with Cyril, we should not put a dependency to Roassal

@hernanmd
Copy link
Contributor Author

Could we have a Baseline group for loading visualizations examples as extension methods and so on?

@jordanmontt
Copy link
Contributor

In my opinion, this should be in the data-inspector repo since we have there other visualizations as well

@SergeStinckwich
Copy link
Member

Can we move all PolyMath visualisations in the PolyMath org?

@jordanmontt
Copy link
Contributor

I'm not against creating a new repo inside PolyMath for visualizations. But, we should be careful to not have duplicated visualizations with data-inspector.
We should discuss this. Maybe in the next ai meeting?

@jecisc
Copy link
Member

jecisc commented May 31, 2023

I'm also not against it. For me data-inspectors is really about visualizations to help take decisions about your data. If we do on for PolyMath, IMA it should be things helping to visualize Polymath related things. Not visualizations to help take decisions over datas

Hernán Morales Durand added 4 commits December 27, 2023 15:34
* master:
  refactor: Rename Variable where we make the roles of the objects clearer in the tests.
  test: clarified the name of a test.
  fix: corrected a comment.
  test: clarified naming using domain language (repeated roots).
  fix: corrected the comment.
  test: added some interesting and missing tests.
  test: changed the example and added a comment to make it clear what the coefficients correspond to.
  test: added a simple test for x^2 + 2x + 1, whose roots are x = -1 and -1
  refactor: Rename Variable - it looks like it is a root.
  stylg: removed redundant comment.
  refactor: Rename Method, and now we have the interface that a client (in this case the test) needs.
  refactor: Inline Method as it has no clients.
  style: corrected a Smalltalk code critique.
  refactor: clarified the name of the method.
  refactor: Inline Method, as it has no clients.
  refactor: Move Method to a more sensible place.
  refactor: made the method a class method. Next we can inline the instance method.
  refactor: Extract Method where a method instantiates a root finding algorithm with the desired precision.
  refactor: migrated the client to no longer use roots message.
  refactor: using Parallel Change, introduced a new message that returns roots in order.
Update baseline dependencies.
Also reduce baseline long method.
Update Roassal deprecated class names.
@hernanmd
Copy link
Contributor Author

I repackaged it in a new package Math-Visualizations and updated the Baseline references.
It also contains a new group visualizations to load PolyMath code related with graphics.

@Ducasse
Copy link
Contributor

Ducasse commented Dec 28, 2023

You have two approaches

  • either another project PMWithVisualization with dependency to roassal and PM
  • or a package and you factor the dependency: you can load some PM packages without the dependency (but the dependency is still at the level of project).

So the second one cost less in terms of management. And I think that in anycase this is cool to have visualizations :)

@hernanmd
Copy link
Contributor Author

Yes, this PR fixes the dependency problem. If you load it normally PM packages are loaded without any visualization dependency:

Metacello new
        repository: 'github://PolyMathOrg/PolyMath:plot_pca';
        baseline: 'PolyMath';
        load

So there is no Roassal dependency (except a Roassal reference from another package, "Math-TSNE", which I will address in another PR).

If you load the branch of this PR, there will be a new package Math-Visualizations:

Metacello new
	repository: 'github://PolyMathOrg/PolyMath:plot_pca';
	baseline: 'PolyMath';
	load: #('visualizations')

I tested under Pharo 11 and Pharo 12 and it works as expected.

Screenshot 2023-12-28 at 20 00 21

It is already safe to be integrated

@jordanmontt
Copy link
Contributor

It looks good for me

@hernanmd hernanmd merged commit 7786dc1 into master Jan 18, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants